Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Form autosave #144

Merged
merged 9 commits into from
May 16, 2024
Merged

Form autosave #144

merged 9 commits into from
May 16, 2024

Conversation

dmijatovic
Copy link
Collaborator

@dmijatovic dmijatovic commented Apr 5, 2024

Save changes in the form automatically. The autosave option is enabled by default.

  • Use autosave state to decide if the changes are saved automatically
  • Default value is true, with autosave the changes are saved automatically
  • If autosave is unchecked Save, Cancel and Delete button are shown at the bottom of the node form, as it was in the previous version
  • Clear button clears configuration and sets the default start values (empty array of molecules)
  • Fixed the upload button after using Clear button.

UI changes:

  • Buttons styles are slightly changed.
  • Download button is disabled if there are errors in the configuration.
  • The error notification is shown for each node in the workflow list (see image 2) using the classes form-control is-invalid.
  • Additional tab is added to workflow for the files. There are 3 tabs in total: Visual, Text and Files
  • Global parameters is the first item of the workflow list. It cannot be removed from the workflow.
  • Default store value is set to empty molecules array
  • Undefined values are removed from the global parameters in order to avoid toml error for the required params (molecules and run directory)

image

Error notification in the workflow

image

Preferred approach instead of #138
Closes #138

Copy link

netlify bot commented Apr 5, 2024

Deploy Preview for i-vresse-workflow-builder ready!

Name Link
🔨 Latest commit a958379
🔍 Latest deploy log https://app.netlify.com/sites/i-vresse-workflow-builder/deploys/6644c41e5706ea00084bd044
😎 Deploy Preview https://deploy-preview-144--i-vresse-workflow-builder.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 48.83721% with 110 lines in your changes are missing coverage. Please review.

Project coverage is 65.61%. Comparing base (ef60923) to head (1417746).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
- Coverage   66.24%   65.61%   -0.63%     
==========================================
  Files          57       57              
  Lines        4076     4235     +159     
  Branches      338      344       +6     
==========================================
+ Hits         2700     2779      +79     
- Misses       1372     1452      +80     
  Partials        4        4              
Flag Coverage Δ
core-unit 62.54% <49.05%> (-0.58%) ⬇️
form-unit 78.41% <33.33%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ackages/core/src/molecule/addMoleculeValidation.ts 92.94% <100.00%> (+0.08%) ⬆️
packages/core/src/molecule/parse.ts 92.85% <100.00%> (+0.75%) ⬆️
packages/core/src/toml.ts 91.80% <100.00%> (+0.10%) ⬆️
packages/form/src/ArrayFieldTemplate.tsx 74.33% <100.00%> (-0.09%) ⬇️
packages/core/src/WorkflowDownloadButton.tsx 0.00% <0.00%> (ø)
packages/core/src/index.tsx 0.00% <0.00%> (ø)
packages/core/src/FormActions.tsx 0.00% <0.00%> (ø)
packages/core/src/VisualPanel.tsx 69.76% <90.47%> (+3.69%) ⬆️
packages/form/src/Form.tsx 77.35% <0.00%> (-3.04%) ⬇️
packages/core/src/GlobalForm.tsx 88.63% <83.33%> (-11.37%) ⬇️
... and 7 more

@dmijatovic dmijatovic force-pushed the form-autosave branch 2 times, most recently from 2ec08c5 to 3a8df4a Compare April 24, 2024 14:33
…adaptations.

tests: fix failing unit and integration tests
@dmijatovic dmijatovic marked this pull request as ready for review April 25, 2024 14:47
@dmijatovic dmijatovic changed the title WIP: Form autosave Form autosave Apr 25, 2024
@VGPReys
Copy link

VGPReys commented Apr 26, 2024

related to #147

Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice ux improvement.

As a user I could not find big problems except the following

Given the initial page loaded of the haddock3-download app then adding the topoaa node by clicking it will mark both the global parameters and topoaa node as invalid in the middle column. I expected to only have global invalid. When I swich back to global and fix the error then the topoaa is still incorrectly shown as invalid.

The application in apps/* which are not haddock3-download are in auto save mode and also have a save button. Which is a bit strange, but not that bad that it needs fixing.

Finding an error in a big form like for guru catalog and emref node can be hard. It would be nice if in CollapsibleField component you could show that child field has an error. This could be done by adding

const hasError = Object.keys(props.errorSchema).length > 0 ? 'text-danger' : ''
...
<div className={'card-header ' + hasError}>

Could you add this to the CollapsibleField and make it look similar to other errors?

As a developer I have added some inline suggestions.

packages/core/src/workflow.ts Outdated Show resolved Hide resolved
packages/core/src/store.ts Outdated Show resolved Hide resolved
packages/core/src/FilesList.tsx Outdated Show resolved Hide resolved
packages/core/src/FilesList.tsx Outdated Show resolved Hide resolved
packages/core/src/GlobalForm.tsx Outdated Show resolved Hide resolved
packages/core/src/NodePanel.tsx Show resolved Hide resolved
packages/core/src/WorkflowDownloadButton.tsx Outdated Show resolved Hide resolved
packages/core/src/index.tsx Outdated Show resolved Hide resolved
packages/core/src/molecule/parse.ts Show resolved Hide resolved
packages/core/src/store.ts Outdated Show resolved Hide resolved
@dmijatovic dmijatovic requested a review from sverhoeven May 6, 2024 13:24
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice , all the apps have consistent autosave. Also linting gives no errors.

I have just a single minor inline suggestion for ts-standard version.

I made some changes to fix topoaa and rsmdmatrix + a workaround in #156

Feel free to review my changes and merge this PR.

packages/form/package.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants